Codecov Report

Merging #207 into master will increase coverage by 0.17%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #207      +/-   ##
==========================================
+ Coverage   90.59%   90.76%   +0.17%     
==========================================
  Files          24       25       +1     
  Lines        1723     1744      +21     
==========================================
+ Hits         1561     1583      +22     
+ Misses        162      161       -1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be0a32e...fc256e1. Read the comment docs.

Thanks for taking this on!

I think my preference would be to just delete the function, but maintain the naming of variables inside our other functions (e.g. still use num_states, num_actions instead of m, n).

I'd like to get some other opinions on that though... cc @oyamad @cc7768 @mmcky @jstac

Thanks for working to close this issue @a-parida12

I agree with @sglyon -- I like the explicitness of num_states and num_actions. Well named variables can often serve as a way of documenting code.

@sglyon I don't fully understand which particular function to be dropped then.

Sorry for not being clear on this one.

You did delete the proper function, but in a few instances you also changed the name of some variables (in the DiscreteDP constructor as well as a few docstrings) away from num_actions to the single letter n.

We should keep the descriptive variable names, but still delete the function as you did. I think that we want to end up with a 1-line change relative to the current master branch that just removes the function definition. We should also check src/QuantEcon.jl to make sure num_actions is not being exported and the test files to make sure it is not called in the tests..

Thank you